feat: use mobilecli ios device#271
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughThe iOS device handler in Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server.ts (1)
154-159:⚠️ Potential issue | 🟠 MajorIncompatible device IDs between go-ios and mobilecli for physical iOS devices.
Physical iOS devices use
IosManager.listDevices()which returns go-ios UDIDs, but these are passed directly toMobileDevicewhich sends them tomobilecli --device. However, simulator handling usesmobilecli.getDevices()and passes mobilecli's own device IDs, suggesting the two tools use incompatible ID formats. This will causeMobileDevicecommands to fail for real iOS devices. Either verify the formats are compatible or usemobilecli.getDevices()withtype: "real"for physical iOS devices instead of querying go-ios.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server.ts` around lines 154 - 159, The code uses IosManager.listDevices() UDIDs and then constructs a MobileDevice(deviceId), but go-ios UDIDs differ from mobilecli device IDs causing MobileDevice commands to fail; update the logic to query mobilecli.getDevices({ type: "real" }) (or call mobilecli.getDevices() and filter real devices) and find the matching device by comparing the mobilecli device id to deviceId, then pass that mobilecli id into new MobileDevice(...) instead of using IosManager.listDevices(); references: IosManager.listDevices(), iosManager, MobileDevice, and mobilecli.getDevices().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/server.ts`:
- Around line 154-159: The code uses IosManager.listDevices() UDIDs and then
constructs a MobileDevice(deviceId), but go-ios UDIDs differ from mobilecli
device IDs causing MobileDevice commands to fail; update the logic to query
mobilecli.getDevices({ type: "real" }) (or call mobilecli.getDevices() and
filter real devices) and find the matching device by comparing the mobilecli
device id to deviceId, then pass that mobilecli id into new MobileDevice(...)
instead of using IosManager.listDevices(); references: IosManager.listDevices(),
iosManager, MobileDevice, and mobilecli.getDevices().
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
src/server.ts
17efa43 to
a35c325
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server.ts (1)
146-183:⚠️ Potential issue | 🔴 CriticalMobileDevice does not fully support physical iOS devices — critical functionality is missing.
The change replaces
IosRobotwithMobileDevicefor physical iOS devices (line 156), butMobileDevicelacks essential capabilities:
- Missing
getIosVersion()method thatIosRobotprovides- No tunnel management or WebDriverAgent infrastructure that physical devices require (especially iOS 17+)
- Different backend:
IosRobotuses go-ios + WebDriverAgent, whileMobileDeviceonly uses mobilecli
MobileDevicewas designed as a unified class for simulators and Android devices using mobilecli, not as a replacement for physical iOS device handling. This change will break physical iOS device operations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server.ts` around lines 146 - 183, getRobotFromDevice currently returns a MobileDevice for physical iOS devices (using IosManager to detect them) but MobileDevice lacks key iOS-specific features (getIosVersion, tunnel/WDA management and go-ios backend) so revert the change: when IosManager finds a physical device return an IosRobot (not MobileDevice); leave MobileDevice for simulators only (the mobilecli branch and simulator detection using mobilecli.getDevices). If you cannot instantiate IosRobot directly, delegate iOS-specific calls from MobileDevice to an IosRobot instance or implement missing methods (getIosVersion, tunnel/WDA setup) in MobileDevice to match IosRobot behavior; update references to IosManager, MobileDevice, IosRobot and getRobotFromDevice accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/server.ts`:
- Around line 146-183: getRobotFromDevice currently returns a MobileDevice for
physical iOS devices (using IosManager to detect them) but MobileDevice lacks
key iOS-specific features (getIosVersion, tunnel/WDA management and go-ios
backend) so revert the change: when IosManager finds a physical device return an
IosRobot (not MobileDevice); leave MobileDevice for simulators only (the
mobilecli branch and simulator detection using mobilecli.getDevices). If you
cannot instantiate IosRobot directly, delegate iOS-specific calls from
MobileDevice to an IosRobot instance or implement missing methods
(getIosVersion, tunnel/WDA setup) in MobileDevice to match IosRobot behavior;
update references to IosManager, MobileDevice, IosRobot and getRobotFromDevice
accordingly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
src/server.ts
a35c325 to
b9eace8
Compare
Summary by CodeRabbit